Skip to content

fix(proxy): recover goal restarts from unavailable owners - #1679

Open
leventov wants to merge 5 commits into
Soju06:mainfrom
leventov:fix/recover-restarted-conversation-affinity
Open

fix(proxy): recover goal restarts from unavailable owners#1679
leventov wants to merge 5 commits into
Soju06:mainfrom
leventov:fix/recover-restarted-conversation-affinity

Conversation

@leventov

@leventov leventov commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

A Codex conversation restart can resend a self-contained thread under the same
process-session identifier after its legacy owner exhausts quota. Raw legacy
codex_session rows are intentionally hard, so ordinary requests must fail
closed; before this change, that same row also trapped an explicit,
self-contained goal restart on an unavailable account.

This PR adds one proof-gated direct-routing exception. A request may retire an
unavailable raw legacy owner only when it carries Codex's recognized
goal-continuation marker and its canonical upstream Responses payload is
account-neutral and self-contained. Retirement is compare-and-set,
policy-scoped, and request-owned. Ordinary, incremental, file-pinned,
conversation-bound, and unresolved-tool requests remain fail-closed.

HTTP bridge reuse/replacement is deliberately split into the dependent #1680.
No public wire format, setting, or default timeout changes. One nullable sticky-session column records source-qualified abandonment without backfilling historical rows.

Linked issue: none exists for this incident-derived defect; routed regression
coverage exercises the public Codex Responses and direct WebSocket paths.

Behavior and safety

  • Recognize the existing goal-continuation context only after canonicalizing the
    request to the upstream Responses body.
  • Permit guarded retirement only for a raw legacy process-session row whose
    owner is durably PAUSED, RATE_LIMITED, or QUOTA_EXCEEDED.
  • Require the persisted owner to be inside authenticated account-assignment and
    security-policy scope, computed before model and service-tier eligibility.
  • Scope abandonment to process-session interpretation while retaining the raw
    account as hard ownership for an equal explicit turn-state value.
  • Compare mapping owner and unavailable status atomically; a concurrent owner
    change or recovery wins.
  • Exclude a successfully retired owner from the remainder of the same selection,
    even when selection loaded a stale ACTIVE account snapshot.
  • Preserve request-scoped authority through direct WebSocket selection and strip
    only proxy-generated turn state when the restart changes accounts.
  • Keep every unproved or account-dependent continuation hard owner-bound.

OpenSpec

  • Behavior is specified by the archived, verified change at
    openspec/changes/archive/2026-08-10-recover-restarted-conversation-affinity/.
  • Stable requirements and rationale are synchronized in
    openspec/specs/sticky-session-operations/.
  • Parent requirements cover canonical classification, scoped CAS retirement,
    stale-selection exclusion, and direct WebSocket state provenance.
  • HTTP bridge requirements are isolated in the stacked fix(http-bridge): preserve goal-restart recovery across reconnects #1680 delta.

Origin and concurrent work

Landed lineage:

Concurrent work reviewed for overlap:

Review findings addressed

Concrete review findings incorporated in this object:

  1. retirement must be limited to the authenticated account-policy scope;
  2. successful retirement must override stale account-selection snapshots;
  3. accepted compatibility and transport-envelope fields must classify through
    the canonical upstream request body;
  4. proxy-generated direct-WebSocket turn state must not cross the account
    change, while client-supplied state remains hard;
  5. raw-key abandonment must retain colliding explicit turn-state ownership;
  6. model and service-tier eligibility must not narrow authenticated mutation
    authority.

Validation

affected suites: 1,195 passed
load-balancer refresh signature regressions: 87 passed, 3 skipped
Ruff check and format: passed
type and architecture/import checks: passed
additive migration upgrade/downgrade regression: passed
OpenSpec validation: 49 specs passed, 0 failed
git diff --check: passed

Fresh current-head cloud CI is in progress after the object-level history split.

Screenshots / output

No dashboard-visible change.

Before:

marked self-contained restart + unavailable raw owner -> hard-affinity failure

After:

verified marked restart -> guarded CAS retirement -> eligible replacement owner
ordinary/account-dependent request -> unchanged fail-closed ownership

Simplicity

  • Zero configuration; no setting or environment variable.
  • One additive nullable migration with no historical backfill; no setup step,
    README section, or dashboard surface.
  • Reuses existing replay classification, sticky tombstones, and selection.
  • No new retry budget or timeout.

Checklist

  • Conventional Commit PR title.
  • OpenSpec behavior is present, synchronized, verified, and archived.
  • Direct route, direct WebSocket, scoped-policy, stale-snapshot,
    source-provenance, model-authority, and migration regressions are included.
  • Simplicity gates P1-P5 reviewed.
  • CHANGELOG.md was not edited.
  • Current-head cloud CI green.
  • Current-head Codex review evidence clean or all findings addressed.

Permit an explicit, account-neutral Codex goal restart to retire an unchanged legacy owner only while its persisted account status is unavailable. Preserve fail-closed routing for all other continuity evidence and guard retirement with one compare-and-set tombstone.
@leventov
leventov force-pushed the fix/recover-restarted-conversation-affinity branch from 2412c6d to a8e8199 Compare August 10, 2026 12:08
@Komzpa

Komzpa commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cd554303b9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/modules/proxy/affinity.py
Comment thread app/modules/proxy/_service/websocket/mixin.py
Comment thread app/modules/proxy/_load_balancer/sticky_selection.py Outdated
@Komzpa Komzpa added the 🤖 codex: needs work [@codex review] raised an issue label Aug 10, 2026
@github-actions github-actions Bot added the db migration PR changes Alembic database migrations; maintainer must coordinate merge order label Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖 codex: needs work [@codex review] raised an issue db migration PR changes Alembic database migrations; maintainer must coordinate merge order

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants